Track bundle resource state sizes in telemetry (direct engine)#5199
Open
shreyas-goenka wants to merge 1 commit into
Open
Track bundle resource state sizes in telemetry (direct engine)#5199shreyas-goenka wants to merge 1 commit into
shreyas-goenka wants to merge 1 commit into
Conversation
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
4a3106b to
715f019
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
715f019 to
77ec7bc
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
77ec7bc to
85dd380
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
85dd380 to
65bb595
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
65bb595 to
44e4bc0
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
changed the title
Contributor
Approval status: pending
|
Collaborator
|
Commit: 1cab854 |
denik
reviewed
Jun 3, 2026
| db := dstate.NewDatabase("", 0) | ||
|
|
||
| pattern := dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey()) | ||
| _, err := dyn.MapByPattern(b.Config.Value(), pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { |
Contributor
There was a problem hiding this comment.
Why walk the config and not the actual state? They might not match 1-1.
Contributor
Author
There was a problem hiding this comment.
To capture the state for both terraform and direct deployments. Most customers are still on terraform so this givess us approximate stats for the state sizes.
Contributor
There was a problem hiding this comment.
I see, in that case we should not call StateFileSize, since it has nothing to do with it, we should call it ConfigFileSize.
Contributor
Author
There was a problem hiding this comment.
It does try to approximate the state size - by calling PrepareState:
target := cfg
if adapter, ok := adapters[resourceType]; ok {
state, err := adapter.PrepareState(cfg)
if err != nil {
return nil, fmt.Errorf("prepare state: %w", err)
}
target = state
}
// dstate.SaveState writes resource state with MarshalIndent using these
// exact prefix/indent arguments; matching them here means each resource's
// byte length equals len(entry.State) on disk for direct deploys.
raw, err := json.MarshalIndent(target, " ", " ")
if err != nil {
return nil, fmt.Errorf("marshal: %w", err)
}
return raw, nil
denik
reviewed
Jun 3, 2026
|
|
||
| var fileSize int64 | ||
| if len(db.State) > 0 { | ||
| raw, mErr := json.MarshalIndent(db, "", " ") |
Contributor
There was a problem hiding this comment.
why not os.Stat actual state file to get the size?
denik
reviewed
Jun 3, 2026
| // dstate.SaveState writes resource state with MarshalIndent using these | ||
| // exact prefix/indent arguments; matching them here means each resource's | ||
| // byte length equals len(entry.State) on disk for direct deploys. | ||
| raw, err := json.MarshalIndent(target, " ", " ") |
Contributor
There was a problem hiding this comment.
this will add some overhead to large bundles (e.g. python-generated one with a lot of jobs).
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
44e4bc0 to
d16c208
Compare
shreyas-goenka
changed the title
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Adds a `resources_metadata` field to the bundle deploy telemetry event with, per resource type, the count and the max/mean/median state size in bytes, plus the whole state file size. Only direct deploys are measured, and collection does no marshalling, file read, or JSON parsing of its own. The direct engine already serializes each resource's state during the deploy and reconstructs it via WAL replay in Finalize; ExportStateFromData now records each entry's len(state) on the ResourceState it returns. deployCore stashes that finalized state on b.Metrics, and telemetry reads the per-resource sizes straight off the in-memory map. The whole-file size comes from a single os.Stat (no read/parse). Terraform stores state differently and is not collected (the field is absent there). Because the metadata is direct-only it diverges across the DATABRICKS_BUNDLE_ENGINE test matrix, so the shared telemetry/deploy golden omits it; the logic is covered by unit tests. The universe proto (resources_metadata, BundleResourcesMetadata, ResourceMetadata) is already merged, so this is ingested rather than dropped. Co-authored-by: Isaac
shreyas-goenka
force-pushed
the
shreyas-goenka/state-size-telemetry
branch
from
d16c208 to
1cab854
Compare
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
resources_metadatafield to the bundle deploy telemetry event with, per resource type, thecountand the max/mean/median state size in bytes, plus the whole state file size.Only direct deploys are measured, and collection does no marshalling, file read, or JSON parsing of its own. The direct engine already serializes each resource's state during the deploy and reconstructs it via WAL replay in
Finalize;ExportStateFromDatanow records each entry'slen(state)on theResourceStateit returns.deployCorestashes that finalized state onb.Metrics, and telemetry reads the per-resource sizes straight off the in-memory map. The whole-file size comes from a singleos.Stat(no read/parse). Terraform stores state differently and is not collected (the field is absent there).Because the metadata is direct-only it diverges across the
DATABRICKS_BUNDLE_ENGINEtest matrix, so the sharedtelemetry/deploygolden omits it; the logic is covered by unit tests.The universe proto (
resources_metadata,BundleResourcesMetadata,ResourceMetadata) is already merged, so this is ingested rather than dropped.This pull request and its description were written by Isaac.